logpuller: send CDC scan priority by hongyunyan · Pull Request #5480 · pingcap/ticdc

hongyunyan · 2026-06-23T02:27:23Z

Background

TiCDC creates different kinds of log puller region tasks. Bootstrap scans for newly created changefeeds should not have the same downstream scan priority as repair or retry scans that affect existing changefeed lag.

Motivation

Propagating scan priority through ChangeDataRequest lets CSE protect high-priority CDC scans while scheduling new changefeed bootstrap scans as low priority.

Summary

Map log puller task types to cdcpb.ScanPriority.
Set ChangeDataRequest.ScanPriority when building CDC requests.
Preserve request priority across busy and congested retry paths.
Add focused tests for task priority mapping, request construction, and retry behavior.
Update github.com/pingcap/kvproto to the protocol branch pseudo-version.
Merge the latest master and preserve event store changefeed IDs when creating log puller subscriptions.

Dependencies

Protocol PR: cdcpb: add CDC scan priority kvproto#1493

Validation

go mod tidy
make fmt
make cdc
NEXT_GEN=1 make cdc
go test ./logservice/eventstore ./logservice/logpuller
tools/bin/golangci-lint run --timeout 10m0s --new-from-rev=2258270f041a2267ede73126a033f889f4d51632

Summary by CodeRabbit

New Features
- Added configuration option old-start-ts-scan-low-priority-threshold (default: 30m) to control initial scan priority for older start positions.
- Range/region requests now include scan priority and changefeed identifier; unknown priorities are normalized to low, and realtime priority is upgraded to high after the subscription catches up.
Bug Fixes
- Improved retry handling to preserve the original scan priority and derive the correct priority for retried work.
- Enhanced scan-priority warning and subscription logs with clearer changefeed identifiers.

ti-chi-bot · 2026-06-23T02:27:26Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

coderabbitai · 2026-06-23T02:27:32Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0d26099e-959f-42f3-9181-46c006b9a0b6

📥 Commits

Reviewing files that changed from the base of the PR and between da6b17a and ab05078.

📒 Files selected for processing (2)

logservice/logpuller/priority_task.go
logservice/logpuller/subscription_client.go

💤 Files with no reviewable changes (1)

logservice/logpuller/priority_task.go

🚧 Files skipped from review as they are similar to previous changes (1)

logservice/logpuller/subscription_client.go

📝 Walkthrough

Walkthrough

This PR adds scan-priority scheduling in logpuller, threads changefeedID through subscription call paths, introduces a configurable old-start-ts threshold, and updates request creation and retry handling to carry scan priority metadata.

Changes

Scan Priority Scheduling and ChangefeedID Threading

Layer / File(s)	Summary
Dependency bump and TaskType/ScanPriority mapping helpers `go.mod`, `logservice/logpuller/priority_task.go`, `logservice/logpuller/priority_task_test.go`	Bumps `kvproto` and adds `TaskType` ↔ `cdcpb.ScanPriority` conversion and normalization helpers with tests.
Configurable old-start-ts low-priority threshold `pkg/config/debug.go`	Adds `DefaultOldStartTsScanLowPriorityThreshold`, `PullerConfig.OldStartTsScanLowPriorityThreshold`, and validation/default wiring.
Region state scanPriority field `logservice/logpuller/region_state.go`, `logservice/logpuller/region_req_cache_test.go`	Adds `scanPriority` to `regionInfo`, initializes it, and updates a test helper to set `changefeedID` on `subscribedSpan`.
Region request creation with scan priority and changefeedId `logservice/logpuller/region_request_worker.go`, `logservice/logpuller/region_request_worker_test.go`	Populates `ChangefeedId` and normalized `ScanPriority` on outgoing requests, adds `changefeedID` to debug logs, and tests request priority mapping.
Subscription client Subscribe, scheduling, and retry priority `logservice/logpuller/subscription_client.go`, `logservice/logpuller/region_event_handler.go`	Updates `Subscribe` to accept `changefeedID`, adds `realtimeScanPriority`, computes initial priority from `startTs`, enables realtime priority after catch-up, and uses derived retry priority in error handling.
Subscription client tests for scan priority and changefeedID `logservice/logpuller/subscription_client_test.go`	Updates existing tests for the new API and adds coverage for retry priority preservation, initial scan priority selection, and realtime priority upgrades.
RegisterDispatcher and DDL fetcher pass changefeedID to Subscribe `logservice/eventstore/event_store.go`, `logservice/eventstore/event_store_test.go`, `logservice/schemastore/ddl_job_fetcher.go`	Names and logs `changefeedID` in `RegisterDispatcher`, passes it into `Subscribe`, updates the mock signature, and formats a DDL subscription name.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested reviewers: asddongmen, lidezhu

Sequence Diagram(s)

sequenceDiagram
  participant DdlJobFetcher
  participant EventStore
  participant SubscriptionClient
  participant RegionEventHandler
  participant RegionRequestWorker

  DdlJobFetcher->>SubscriptionClient: Subscribe(changefeedID, span, startTs)
  EventStore->>SubscriptionClient: Subscribe(changefeedID, span, startTs)
  SubscriptionClient->>SubscriptionClient: newSubscribedSpan(changefeedID, ...)
  SubscriptionClient->>SubscriptionClient: initialScanTaskPriority(startTs)
  RegionEventHandler->>SubscriptionClient: maybeEnableRealtimeScanPriority(span, resolvedTs)
  SubscriptionClient->>RegionRequestWorker: scheduleRegionRequest(effectiveScanTaskPriority)
  RegionRequestWorker->>RegionRequestWorker: createRegionRequest(ChangefeedId, ScanPriority)
  SubscriptionClient->>SubscriptionClient: doHandleError(retryPriority)

Poem

A rabbit hops on priority toes,
High and low in a tidy pose.
ChangefeedID trails each little run,
Realtime wakes when catch-up’s done.
🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the change, but it does not follow the repository’s required PR template sections.	Add the required template sections, especially Issue Number, checklist/tests, compatibility/docs questions, and a release-note block.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately captures the main change: propagating CDC scan priority in logpuller.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

gemini-code-assist

Code Review

This pull request updates the kvproto dependency and introduces support for preserving scan priority across retries when handling congested or busy server errors in logpuller. It adds mapping utilities between TaskType and cdcpb.ScanPriority, stores the scan priority in regionInfo, and ensures it is propagated and maintained during retries. Comprehensive unit tests are also added to verify these mappings and retry behaviors. I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Signed-off-by: hongyunyan <649330952@qq.com>

ti-chi-bot · 2026-06-27T02:27:33Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nongfushanquan for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS
pkg/config/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: hongyunyan <649330952@qq.com>

coderabbitai

🧹 Nitpick comments (1)

logservice/logpuller/subscription_client_test.go (1)
537-731: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Well-structured coverage of scan-priority behavior; minor duplication across new tests.

Assertions for TestInitialScanTaskPriority, TestSubscribeUsesInitialScanTaskPriority, TestRealtimeScanPriorityEnabledAfterSubscriptionCatchesUp, and the retry-upgrade tests all match the priority computation/upgrade logic shown in subscription_client.go (initialScanTaskPriority, maybeEnableRealtimeScanPriority, effectiveScanTaskPriority, doHandleError).

The rawSpan/subscribedSpan/regionInfo construction block (Lines 406-417, 480-491, 666-678, 695-707) is duplicated near-verbatim across four test functions. Consider extracting a small helper (e.g. newTestRegionWithScanPriority(t, subID, priority)) to reduce duplication and ease future maintenance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@logservice/logpuller/subscription_client_test.go` around lines 537 - 731, The
new scan-priority tests repeat the same raw span, subscribed span, and region
setup in several places, so factor that shared construction into a small helper
to keep the tests maintainable. Extract the repeated setup around
TestInitialScanTaskPriority, TestSubscribeUsesInitialScanTaskPriority,
TestRealtimeScanPriorityEnabledAfterSubscriptionCatchesUp, and the retry-upgrade
tests into a helper such as newTestRegionWithScanPriority or
newTestSubscribedSpan, then reuse it while keeping each test focused on its
specific priority assertion.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@logservice/logpuller/subscription_client_test.go`:
- Around line 537-731: The new scan-priority tests repeat the same raw span,
subscribed span, and region setup in several places, so factor that shared
construction into a small helper to keep the tests maintainable. Extract the
repeated setup around TestInitialScanTaskPriority,
TestSubscribeUsesInitialScanTaskPriority,
TestRealtimeScanPriorityEnabledAfterSubscriptionCatchesUp, and the retry-upgrade
tests into a helper such as newTestRegionWithScanPriority or
newTestSubscribedSpan, then reuse it while keeping each test focused on its
specific priority assertion.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2d06ba2b-1f41-4995-9b2f-4421ea62b717

📥 Commits

Reviewing files that changed from the base of the PR and between 2258270 and 3a26268.

⛔ Files ignored due to path filters (1)

go.sum is excluded by !**/*.sum

📒 Files selected for processing (14)

go.mod
logservice/eventstore/event_store.go
logservice/eventstore/event_store_test.go
logservice/logpuller/priority_task.go
logservice/logpuller/priority_task_test.go
logservice/logpuller/region_event_handler.go
logservice/logpuller/region_req_cache_test.go
logservice/logpuller/region_request_worker.go
logservice/logpuller/region_request_worker_test.go
logservice/logpuller/region_state.go
logservice/logpuller/subscription_client.go
logservice/logpuller/subscription_client_test.go
logservice/schemastore/ddl_job_fetcher.go
pkg/config/debug.go

Signed-off-by: hongyunyan <649330952@qq.com>

logpuller: send cdc scan priority

a1207a5

ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 23, 2026

gemini-code-assist Bot reviewed Jun 23, 2026

View reviewed changes

hongyunyan added 2 commits June 26, 2026 16:14

logpuller: preserve scan priority for retries

638d564

logservice: add changefeed scan task logs

4e95247

Signed-off-by: hongyunyan <649330952@qq.com>

ti-chi-bot Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 26, 2026

logpuller: classify initial scan priority by start ts

367bf9c

Signed-off-by: hongyunyan <649330952@qq.com>

logpuller: protect realtime recovery scans

d591557

Signed-off-by: hongyunyan <649330952@qq.com>

ti-chi-bot Bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 29, 2026

hongyunyan added 3 commits July 7, 2026 13:39

logpuller: merge latest master for scan priority

efc23de

Signed-off-by: hongyunyan <649330952@qq.com>

logservice: keep changefeed id in event store merge

871a7ff

Signed-off-by: hongyunyan <649330952@qq.com>

logservice: modernize priority scan tests

3a26268

Signed-off-by: hongyunyan <649330952@qq.com>

hongyunyan marked this pull request as ready for review July 7, 2026 06:46

ti-chi-bot Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 7, 2026

hongyunyan added release-note-none Denotes a PR that doesn't merit a release note. skip-issue-check Indicates that a PR no need to check linked issue. labels Jul 7, 2026

ti-chi-bot Bot removed do-not-merge/needs-linked-issue do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 7, 2026

coderabbitai Bot reviewed Jul 7, 2026

View reviewed changes

hongyunyan added 3 commits July 7, 2026 16:05

logpuller: simplify scan priority scheduling

51619e1

Signed-off-by: hongyunyan <649330952@qq.com>

logpuller: simplify priority scan diagnostics

da6b17a

Signed-off-by: hongyunyan <649330952@qq.com>

logpuller: simplify task priority logging

ab05078

Signed-off-by: hongyunyan <649330952@qq.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

logpuller: send CDC scan priority#5480

logpuller: send CDC scan priority#5480
hongyunyan wants to merge 11 commits into
pingcap:masterfrom
hongyunyan:codex/cdc-scan-priority-end-to-end

hongyunyan commented Jun 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

ti-chi-bot Bot commented Jun 23, 2026

Uh oh!

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

ti-chi-bot Bot commented Jun 27, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hongyunyan commented Jun 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Motivation

Summary

Dependencies

Validation

Summary by CodeRabbit

Uh oh!

ti-chi-bot Bot commented Jun 23, 2026

Uh oh!

coderabbitai Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

ti-chi-bot Bot commented Jun 27, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hongyunyan commented Jun 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading